Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/expo/app/`(app)/ai-chat.tsx:
- Around line 93-94: The code currently builds a token variable from
authClient.useSession() and may send "Authorization: Bearer null" when no
session exists; change the request header construction so the Authorization
header is only added when token is a non-null/non-undefined string (i.e. check
_authSession?.session?.token truthiness before adding the header). Update places
referencing the token (the token const and the remote chat request headers
around the fetch/axios calls at lines ~136-138) to conditionally include
Authorization and omit the header entirely when token is null.

In `@apps/expo/features/auth/hooks/useAuthInit.ts`:
- Around line 48-55: session.user.role is being force-cast and may contain
unexpected values; before calling userStore.set, normalize/clamp the role to the
supported union ('USER' | 'ADMIN') at runtime. In the useAuthInit flow, derive a
local normalizedRole from (session.user as Record<string, unknown>).role by
checking if it's exactly 'ADMIN' otherwise defaulting to 'USER', and then pass
normalizedRole into userStore.set instead of the direct cast; update references
around userStore.set and the session.user.role usage accordingly.

In `@apps/expo/lib/auth-client.ts`:
- Around line 9-18: The storage adapter for expoClient is using storagePrefix
'packrat' which yields colon-containing keys that expo-secure-store rejects and
it also lacks removeItem; update the storage object passed to expoClient to (1)
sanitize keys before calling SecureStore by converting or encoding illegal
characters (e.g., replace ':' with '_' or percent-encode) so keys only contain
alphanumeric, '.', '-', '_' and (2) implement removeItem to call
SecureStore.deleteItem/deleteItemAsync for the same sanitized key; make these
changes where the storage object with setItem/getItem is defined (the expoClient
storage config) so both storing, retrieving and deleting use the same
sanitized-key function.

In `@packages/api/drizzle/0038_uuid_pk_better_auth_migration.sql`:
- Around line 38-66: Add non-unique indexes on the foreign key columns to avoid
table scans: in the migration that creates "session" and "account", add CREATE
INDEX statements for session.user_id (e.g., session_user_id_idx) and
account.user_id (e.g., account_user_id_idx). Ensure the indexes are created
after each table definition in the same migration so user-scoped lookups on
"session.user_id" and "account.user_id" use the new indexes.

In `@packages/api/package.json`:
- Line 82: Remove the duplicated dependency entry for "drizzle-orm" by deleting
the copy under devDependencies so only the single entry under dependencies
remains; edit package.json to remove the "drizzle-orm" key/value from the
devDependencies object, save, and then run your package manager's install
(npm/yarn/pnpm) to update the lockfile and ensure no leftover duplicates.
- Around line 72-80: The three runtime auth packages are currently in
devDependencies and must be moved to production dependencies: remove
"@better-auth/drizzle-adapter", "better-auth", and "better-auth-cloudflare" from
devDependencies and add them under dependencies with their existing version
strings (preserve versions like "^1.6.9" and "^0.3.0"); update package.json so
runtime imports in src/auth/index.ts and the middleware can be bundled during
production installs and ensure package-lock or yarn lock is updated by running
the install command after the change.

In `@packages/api/src/auth/index.ts`:
- Around line 109-114: The current emailVerification.sendVerificationEmail
handler logs the full one-time verification URL (exposing tokens); change it to
avoid emitting the URL in non-development environments by either passing the URL
directly to the email service/mailer (e.g., call the mailer with { user, url })
and removing the console.log, or if a log is necessary emit a redacted message
(e.g., show only user.email and a note that a verification email was sent)
guarded by an environment check (NODE_ENV === 'development' or isDev). Update
the emailVerification.sendVerificationEmail function to hand the URL to the
mailer and replace any full-URL logs with a redacted or environment-gated log.
- Line 48: The current catch(() => null) on the generateAppleClientSecret call
hides real errors; change the logic in packages/api/src/auth/index.ts so you
only return null when Apple credentials are actually missing (e.g., check the
relevant env fields first), and otherwise call await
generateAppleClientSecret(env) without swallowing exceptions so misconfiguration
errors propagate; alternatively, catch and inspect the thrown error and rethrow
for any error that is not an explicit "missing credentials" condition. Ensure
references: generateAppleClientSecret, appleClientSecret, and env.
- Around line 153-154: Update the Better Auth client configuration so
trustedOrigins includes all frontend origins, not just env.BETTER_AUTH_URL:
locate the trustedOrigins array used when initializing the Better Auth client
(symbol: trustedOrigins) and replace the single-entry list with the same origin
patterns used in the Elysia CORS config (include env.BETTER_AUTH_URL, the
packrat.world host and subdomain regexes, localhost with port regex, the exp://
scheme, and 'https://admin.packratai.com') so requests from web, subdomains,
localhost, Expo and admin are accepted.

In `@packages/api/src/index.ts`:
- Around line 84-89: enrichEnv() currently returns a new Env object when
OSM_HYPERDRIVE is present which breaks per-isolate caching in getEnv() and
getAuth() (they cache by object identity); change enrichEnv() so it does not
create a fresh object on every call — either mutate and reuse the incoming env
(e.g. set env.OSM_DATABASE_URL) or maintain a WeakMap keyed by the original env
to store and return a single enrichedEnv per-isolate; ensure the same enriched
object is returned on subsequent calls so getEnv() and getAuth() can effectively
reuse the cached instances for that isolate.
- Around line 96-101: The current check uses
url.pathname.startsWith('/api/auth') which also matches routes like
'/api/authentication'; update the condition used before calling
getEnv/getAuth/auth.handler to only match the exact '/api/auth' path or the
'/api/auth/' prefix (e.g., change the test to url.pathname === '/api/auth' ||
url.pathname.startsWith('/api/auth/')). Keep the same call sites (getEnv(),
getAuth(validatedEnv), and auth.handler(request)) so only true auth routes are
routed to the Better Auth handler.

In `@packages/api/src/routes/admin/index.ts`:
- Around line 76-83: The CF Access branch currently allows any holder of the CF
Access JWT to bypass admin checks; change it so CF Access only augments, not
replaces, the admin auth: keep using
verifyCFAccessRequest(CF_ACCESS_TEAM_DOMAIN, CF_ACCESS_AUD) but do NOT return
true solely on cfIdentity — instead require the route to be protected by the
authPlugin macro (isAuthenticated: true) and then either (a) map cfIdentity to
an explicit admin allowlist/role (implement a small helper like isCFAdmin that
checks cfIdentity.email or cfIdentity.sub against your admin list/env var) and
combine that with your existing short‑lived admin bearer validation, or (b)
require both a valid short‑lived admin bearer token and a valid cfIdentity
before allowing access; update the /api/admin/** guard to call
verifyCFAccessRequest and then perform the admin role/allowlist check rather
than returning early.

In `@packages/api/src/routes/catalog/index.ts`:
- Around line 320-327: Extract the duplicated 7-line validation into a shared
helper (e.g., export function validateCatalogItemId(itemId: number): boolean)
that performs Number.isFinite, Number.isInteger, >0 and <=2147483647 checks and
returns true/false; then replace each copy in the route handlers with a call to
this helper and keep the existing response behavior (if
validateCatalogItemId(...) is false then return status(404, { error: 'Catalog
item not found' })). Locate replacements by searching for the exact validation
conditions or the literal error response to update all four occurrences.
- Around line 248-253: The runtime guard in the POST handler that manually
checks data.name, data.weight, and data.weightUnit (and the weight <= 0 check)
is dead code because CreateCatalogItemRequestSchema (z.string().min(1),
z.number().positive(), z.enum(WEIGHT_UNITS)) already validates these before the
handler runs; remove the block that returns status(400, ...) and the weight <= 0
check to clean up noise, leaving the handler to rely on the schema validation
and keep existing behavior via CreateCatalogItemRequestSchema and the handler
function that uses status(...) unchanged.

In `@packages/api/src/routes/packTemplates/index.ts`:
- Line 390: The response body spreads newTemplate which can be undefined; call
assertDefined on the result returned from the .returning() call (the const
[createdTemplate] / newTemplate variable) before using it, or otherwise throw a
500/appropriate error if undefined, then return status(201, {
...createdTemplate, items: insertedItems }); follow the same assertDefined
pattern used earlier in this file (see other uses at lines ~190 and ~280) to
ensure the created template is present before spreading.

In `@packages/api/src/schemas/catalog.ts`:
- Line 219: The schema change removed .positive() from
UpdateCatalogItemRequestSchema.weight which weakens validation and breaks
OpenAPI/type inference; restore the positive constraint by adding .positive()
back onto the weight field in UpdateCatalogItemRequestSchema (keep the existing
route-level guard if you want defense-in-depth, or remove the redundant route
check only if you intentionally rely solely on the schema), ensuring the schema
again enforces weight > 0 for any consumers and the generated docs/types reflect
that rule.

In `@packages/api/src/utils/__tests__/env-validation.test.ts`:
- Around line 17-19: The APPLE_PRIVATE_KEY test fixture contains a real-format
PEM private key which triggers secret scanners; replace its value with a
clearly-invalid stub (e.g., "-----BEGIN PRIVATE KEY-----INVALID-TEST-KEY-----END
PRIVATE KEY-----") or a generated dummy string and add an explicit inline
comment near APPLE_PRIVATE_KEY and APPLE_KEY_ID stating this is a non-production
test stub and has never been registered with Apple so scanners and reviewers
know it is safe; update any tests that validate format to accept the stub or
mock the validation accordingly.

In `@packages/api/test/upload.test.ts`:
- Around line 26-30: Unskip the test named "requires auth for direct upload" and
change its assertion to explicitly verify the current route contract for POST
/upload (expecting 404 or 405) instead of calling expectUnauthorized; update the
test to call api('/upload', httpMethods.post({})) and assert res.status is 404
or 405 (or use an appropriate helper like expectStatus if available) so the test
fails if a POST /upload route is later introduced unexpectedly.

In `@packages/api/wrangler.jsonc`:
- Around line 31-37: The KV namespace placeholders in kv_namespaces (binding
"AUTH_KV") use invalid IDs ("TODO_replace_with_auth_kv_namespace_id" and
"TODO_replace_with_auth_kv_preview_namespace_id"); create real namespaces with
wrangler (e.g., wrangler kv namespace create AUTH_KV and wrangler kv namespace
create AUTH_KV --preview) and replace the two placeholder strings in the
kv_namespaces array with the returned id and preview_id respectively so the
AUTH_KV binding resolves at deploy/runtime.

In `@packages/mcp/src/__tests__/auth.test.ts`:
- Around line 282-315: Replace the manual save/restore of globalThis.fetch in
the test with Vitest's spy: use vi.spyOn(globalThis,
'fetch').mockResolvedValue(...) instead of assigning vi.fn() to globalThis.fetch
and remove the manual restore line; ensure the mocked Response remains the same
and keep the rest of the test flow that calls worker.fetch and asserts
env.OAUTH_KV.put so the spy automatically cleans up after the test (or call
mockRestore() on the spy if you want explicit restore).

In `@packages/mcp/src/auth.ts`:
- Around line 274-281: Wrap the JSON.parse of oauthReqStr and sessionStr in a
try/catch before calling OAuthStateSchema.safeParse and
SessionKvSchema.safeParse so a thrown parse error returns the same 400
invalid_request response; specifically, catch errors around parsing
oauthReqStr/sessionStr used by OAuthStateSchema and SessionKvSchema (the block
that produces oauthReqResult and sessionResult) and on any parse exception
return Response.json({ error: 'invalid_request', error_description: 'Corrupted
state data' }, { status: 400 }) to match the existing validation failure branch.

In `@packages/mcp/wrangler.jsonc`:
- Around line 15-21: The kv_namespaces block contains placeholder IDs that will
break deployment; create real Cloudflare KV namespaces for the binding
"OAUTH_KV" (one for prod and one for dev/preview) and replace the
"__TODO_OAUTH_KV_PROD_ID__" value in the "id" field and the
"__TODO_OAUTH_KV_DEV_ID__" value in the "preview_id" field with the actual
32-character hex IDs returned by the namespace creation commands so the
"kv_namespaces" -> binding "OAUTH_KV" is properly bound for both environments.

---

Outside diff comments:
In `@apps/expo/features/auth/hooks/useAuthActions.ts`:
- Around line 74-103: The signInWithGoogle function can leave isLoading true if
an early return/conditional prevents the catch from running; update
signInWithGoogle to always call setIsLoading(false) in a finally block
(mirroring signIn and signInWithApple) so loading resets regardless of
success/failure or the applySession(data.user) conditional; locate
signInWithGoogle, applySession, and setIsLoading and move the
setIsLoading(false) into a finally after the try/catch to guarantee cleanup.

In `@packages/api/src/routes/admin/index.ts`:
- Around line 363-369: The admin delete handler currently calls db.delete(users)
in the async ({ params }) => { ... } route; change this to perform a soft-delete
update instead: use createDb() and replace the hard delete with an update (e.g.,
db.update(users).where(eq(users.id, id)).set({ is_active: false, deactivated_at:
new Date() })) so the user row is retained for auditability, and separately
revoke or expire sessions/tokens for that user (call your session revoke
function or delete from sessions table) rather than removing the user row;
ensure the handler still returns 404 when no rows are affected and keep the
existing status responses.

In `@packages/api/src/services/__tests__/packService.test.ts`:
- Around line 53-73: The mock factory makePackRow is using a numeric userId
(userId: 1) while tests instantiate PackService with string IDs (e.g.,
'user-test-id-1'), which can cause false positives; update makePackRow to use a
string userId by default and change any inline item mocks (the inline item mock
referenced in the test file) to use matching string userId values so all mocks
mirror the migrated string type expected by PackService.

In `@packages/api/src/services/executeSqlAiTool.ts`:
- Around line 7-15: The Params interface declares userId but executeSqlAiTool
currently ignores it (const { query, limit = 100 } = params); either remove
userId from Params and update callers (e.g., tools.ts) to stop passing it, or
wire it into executeSqlAiTool by extracting userId (const { query, limit = 100,
userId } = params) and performing minimal audit usage such as a
processLogger/console.log entry or attaching it to any DB/audit call before
running createReadOnlyDb() and the query; update the Params definition and
callers accordingly depending on which option you choose.

In `@packages/api/src/utils/__tests__/env-validation.test.ts`:
- Around line 173-176: The test 'throws on missing required variable' currently
unsets JWT_SECRET which is no longer in the schema, so validateCloudflareApiEnv
doesn't throw; update the test to unset the current required variable
(BETTER_AUTH_SECRET) by calling makeRawEnv({ BETTER_AUTH_SECRET: undefined })
and keep the expect(() => validateCloudflareApiEnv(invalid)).toThrow(); also
update the test description to reflect which variable is being omitted;
reference makeRawEnv and validateCloudflareApiEnv when making the change.

In `@packages/api/test/admin-auth-guard.test.ts`:
- Around line 36-38: The JSDoc above the test incorrectly names JWT_SECRET even
though the code now uses BETTER_AUTH_SECRET; update the comment to reference
BETTER_AUTH_SECRET (and any related issuer/audience notes) so it matches the
implementation used by the test harness (search for the JSDoc near the admin
Bearer JWT helper in admin-auth-guard.test.ts and replace JWT_SECRET with
BETTER_AUTH_SECRET).

In `@packages/api/test/admin-jwt.test.ts`:
- Around line 12-13: Update the stale header comment that says "JWT_SECRET is
'secret'" to reflect that the secretKey() helper now reads BETTER_AUTH_SECRET;
locate the comment near the top of packages/api/test/admin-jwt.test.ts and
change the JWT_SECRET reference to BETTER_AUTH_SECRET (or otherwise mention
secretKey() reads BETTER_AUTH_SECRET) so the file header matches the current
implementation used by secretKey().

In `@packages/api/test/setup.ts`:
- Around line 698-721: The tests currently truncate many tables via the
tablesToClean array in setup.ts but omit the jwks table, leaving persisted
signing keys between tests; update the tablesToClean array (used in the test
setup/cleanup logic) to include 'jwks' so the jwks table is truncated between
tests and removes shared auth state.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/api/drizzle/0040_uuid_pk_better_auth_migration.sql`:
- Around line 6-8: When adding "new_id" to users, set a default UUID before
making it NOT NULL: alter the ALTER TABLE ... ADD COLUMN "new_id" text step to
also set DEFAULT gen_random_uuid()::text (or run an ALTER TABLE ... ALTER COLUMN
"new_id" SET DEFAULT gen_random_uuid()::text immediately after populating
existing rows), then make "new_id" NOT NULL; after the PK swap you can remove
the default with ALTER TABLE ... ALTER COLUMN "new_id" DROP DEFAULT. This
ensures INSERTs using the legacy shape won’t fail between migration steps;
reference the "new_id" column and the "users" table and the existing UPDATE
"users" SET "new_id" = gen_random_uuid()::text operation when applying the
change.

In `@packages/api/drizzle/0042_migrate_auth_data.sql`:
- Around line 4-29: The INSERTs into "account" assume legacy nullable
timestamps; update both INSERT INTO "account" statements to COALESCE nullable
sources into safe defaults (e.g., COALESCE(u."created_at", now()) for created_at
and COALESCE(u."updated_at", COALESCE(u."created_at", now())) for updated_at in
the first INSERT, and COALESCE(ap."created_at", u."created_at", now()) for
created_at and COALESCE(ap."created_at", u."created_at", now()) or
COALESCE(ap."updated_at", u."updated_at", now()) for updated_at in the second
INSERT) so no NULLs are inserted into the account.created_at/account.updated_at
NOT NULL columns; apply these COALESCE changes to the INSERTs referencing
"users" u and "auth_providers" ap.

In `@packages/api/drizzle/0044_complete_uuid_conversion.sql`:
- Around line 9-23: Dropping trail_condition_reports.user_id removed the
trail_condition_reports_user_id_idx and the migration never recreates it after
renaming user_uuid back to user_id; add a statement immediately after ALTER
TABLE "trail_condition_reports" RENAME COLUMN "user_uuid" TO "user_id" to
recreate the index (e.g., CREATE INDEX trail_condition_reports_user_id_idx ON
trail_condition_reports(user_id)) so user-scoped queries use the index again.

In `@packages/api/drizzle/0045_finalize_users_uuid_pk.sql`:
- Around line 29-32: The DROP TABLE statements for "auth_providers",
"refresh_tokens", and "one_time_passwords" are executed immediately and remove
the rollback source; postpone removing these legacy tables until after the 24h
validation window by moving or gating these drops out of the immediate migration
(e.g., place them in a separate cleanup script or wrap them behind a
conditional/feature-flag executed after the validation period) so the data
remains available to replay or roll back the Better Auth migration; update any
references in this migration (the DROP TABLE lines for auth_providers,
refresh_tokens, one_time_passwords) to instead defer execution and document the
follow-up cleanup procedure.

---

Outside diff comments:
In `@packages/api/wrangler.jsonc`:
- Around line 129-238: The dev env block (env.dev) omits kv_namespaces so
AUTH_KV is unbound and ValidatedEnv (typed as KVNamespace) will cause the Worker
to crash; fix by creating a dev-specific KV namespace and adding a kv_namespaces
entry under env.dev that binds AUTH_KV to the new namespace_id (e.g., run
`wrangler kv namespace create AUTH_KV --env dev` to get the id) and add that
namespace mapping to the env.dev block so AUTH_KV is present for `wrangler
deploy --env dev`.